Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meilisearch double quote on "match" query #29740

Merged
merged 12 commits into from
Mar 16, 2024

Conversation

6543
Copy link
Member

@6543 6543 commented Mar 12, 2024

make nonFuzzyWorkaround unessesary

cc @Kerollmops

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 12, 2024
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 12, 2024
@6543
Copy link
Member Author

6543 commented Mar 12, 2024

Thank you for the clear explanation. So, exposing the typoTolerance.enabled setting on the search route could fix the issue?

PS: yes t hat would be even better ...

@Kerollmops
Copy link

Would that work?

Yeah, that's the idea. The only downside I see with this "double-quote everything" solution is that you cannot parse the same way as Meilisearch, and that's why the "expose the typoTolerance.enabled setting in the search route" is better in my opinion.

I just talked about this to our Product manager and they will prioritize this.

However, we don't refer to typo tolerance as fuzziness so I was a bit confused 😄

@6543
Copy link
Member Author

6543 commented Mar 12, 2024

@Kerollmops that's awesome ❤️

I hope to get the code-search ready for meilisearch too and at that point we should have a really good integration 🚀

@6543
Copy link
Member Author

6543 commented Mar 12, 2024

well with this patch if I search for Tabellenspalte in the code:

... in der `Tabellenspalte "D"` ist ... 

it wont find it anymore but I think that's ok as #29701 will let the user decide if he want it fuzzy (typoTolerance) or not

thanks in advance to @Kerollmops for pointing out how to tell meilisearch to do so

PS: a space in the example case would fix it:

... in der ` Tabellenspalte "D"` ist ... 

@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 12, 2024
@6543 6543 marked this pull request as ready for review March 12, 2024 15:58
@6543 6543 requested a review from a team March 12, 2024 16:01
@6543 6543 added the type/bug label Mar 12, 2024
@6543 6543 added this to the 1.22.0 milestone Mar 12, 2024
@6543 6543 added the backport/v1.21 This PR should be backported to Gitea 1.21 label Mar 12, 2024
@6543
Copy link
Member Author

6543 commented Mar 12, 2024

@Kerollmops for the ` issue I have created meilisearch/charabia#275 but I could not jet verify it will solfe it. would be nice if you could do a sanity check or tell me how i can test it :)

-> #29740 (comment)

@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Mar 13, 2024
@Kerollmops
Copy link

Thank you for the PR @6543. We will take care of it later this or next week 🙂

@lunny
Copy link
Member

lunny commented Mar 14, 2024

Can you have a ref doc link for the quote behaviours?

meili-bors bot added a commit to meilisearch/charabia that referenced this pull request Mar 14, 2024
275: Support markdown formatted codeblocks r=ManyTheFish a=6543

# Pull Request

## Related issue
Fixes go-gitea/gitea#29740 (comment)

## What does this PR do?
add ``` ` ``` as seperator to support markdown formated text

## PR checklist
Please check if your PR fulfills the following requirements:
> Does this PR fix an existing issue, or have you listed the changes applied in the PR description...

well I can file a issue if desired but I think this is a minor change?

- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes
      -> I hope so ;)


Co-authored-by: 6543 <[email protected]>
@Kerollmops
Copy link

Here it is: https://www.meilisearch.com/docs/reference/api/search#phrase-search

@6543 6543 requested a review from lunny March 14, 2024 12:50
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 14, 2024
@6543 6543 requested a review from lunny March 15, 2024 22:59
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 16, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 16, 2024
@lunny lunny enabled auto-merge (squash) March 16, 2024 03:48
@lunny lunny merged commit c6e5ec5 into go-gitea:main Mar 16, 2024
26 checks passed
@GiteaBot
Copy link
Collaborator

I was unable to create a backport for 1.21. @6543, please send one manually. 🍵

go run ./contrib/backport 29740
...  // fix git conflicts if any
go run ./contrib/backport --continue

@GiteaBot GiteaBot added backport/manual No power to the bots! Create your backport yourself! and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Mar 16, 2024
@6543 6543 deleted the try-to-rm-workaround branch March 16, 2024 13:46
6543 added a commit to 6543-forks/gitea that referenced this pull request Mar 16, 2024
@6543 6543 added the backport/done All backports for this PR have been created label Mar 16, 2024
@6543
Copy link
Member Author

6543 commented Mar 16, 2024

#29846

zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 16, 2024
* giteaofficial/main: (28 commits)
  Forbid jQuery `.prop` and fix related issues (go-gitea#29832)
  Fix wrong test for TestPullView_CodeOwner (go-gitea#29838)
  Forbid HTML injection using jQuery (go-gitea#29843)
  Meilisearch double quote on "match" query (go-gitea#29740)
  Forbid variables containing jQuery collections not having the `$` prefix (go-gitea#29839)
  Remove AddParamIfExist(AddParam) (go-gitea#29841)
  Refactor markdown attention render (go-gitea#29833)
  Refactor code_indexer to use an SearchOptions struct for PerformSearch (go-gitea#29724)
  Refactor AddParam to AddParamIfExist (go-gitea#29834)
  Forbid jQuery AJAX (go-gitea#29818)
  Remove jQuery AJAX from the notifications (go-gitea#29817)
  Light theme color enhancements (go-gitea#29830)
  Better highlighting of archved labels (go-gitea#29749)
  Remove the `time-since` class (go-gitea#29826)
  Remove jQuery AJAX from the project page (go-gitea#29814)
  Upgrade `htmx` to v1.9.11 (go-gitea#29821)
  Dark theme color enhancements (go-gitea#29822)
  Remove jQuery AJAX from the comment edit box (go-gitea#29812)
  Tweak labeler (go-gitea#29809)
  Fix `for` attribute not pointing to the ID of the color picker (go-gitea#29813)
  ...

# Conflicts:
#	routers/web/user/home.go
silverwind pushed a commit that referenced this pull request Mar 16, 2024
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request Mar 22, 2024
Backport go-gitea/gitea#29740 (based on #29671
...)

(cherry picked from commit 0cbbcf20e3f83413a88fe3d436451d707639fe55)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jun 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants